Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 11 and 18 to AN_REQUIRING_PATTERNS, fix for 110 #17

Closed
wants to merge 1 commit into from
Closed

Add 11 and 18 to AN_REQUIRING_PATTERNS, fix for 110 #17

wants to merge 1 commit into from

Conversation

soooh
Copy link

@soooh soooh commented Sep 29, 2016

I was using this library to generate descriptions based on ages and found that 18 and 110 were incorrectly indefinitized:

"18-year-old woman".indefinitize   #=> "a 18-year-old woman"   (incorrect)
"180-year-old statue".indefinitize #=> "a 180-year-old statue" (correct)
"11-year-old house".indefinitize   #=> "an 11-year-old house"  (correct)
"110-year-old house".indefinitize  #=> "an 110-year-old house" (incorrect)

I edited the regex pattern for11to include \b word boundaries so that 110 could be excluded, and included a similar pattern for 18 to correct the cases above.

With this commit, the \b boundary would include punctuation for formatted numbers such as 18,000 but not 18000:

"18,000-year-old artifact".indefinitize  #=> "an 18,000-year-old artifact"
"18.000-year-old artifact".indefinitize  #=> "an 18.000-year-old artifact"
"18000-year-old artifact".indefinitize   #=> "a 18000-year-old artifact"

I saw pull request #16 and the regex pattern given in the July 27 comment looks like it could work, as well, but I'm unsure of the status of that PR. That pattern could be a better solution, but I don't know what cases the last block with digit boundaries would match for.

I'd value your feedback if this isn't an appropriate fix, and would be interested in a discussion of a better approach. Thanks!

@rossmeissl
Copy link
Owner

Thanks @soooh! Could you add the test cases you describe above to the library's test suite? This looks great!

@soooh
Copy link
Author

soooh commented Sep 29, 2016

Hm, I just realized the last digit boundaries that @jongaegler mentions covers groups of 3 digits trailing 18 and 11, and thus would correct indefinitize an unformatted number like 18000. I will test that out on some other cases, and take a look at the test suite, as well.

@rossmeissl
Copy link
Owner

@soooh just checking in on this! Thanks!

@soooh soooh closed this by deleting the head repository Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants